[US-2010] Upgrade to Wails v3.0.0 stable with GTK4 on Linux - #40

Review notes below. The first three matter most: the ContextMenu key double-open, the Linux splash timeout, and the drive-relative embedded file name.

Verdict: COMMENT - 0 must-fix, 1 new should-fix (F1); of anovik's 10 open comments, 6 hold as should-fix and 4 as nits (one of them demoted)

This PR moves the desktop app from Wails v3.0.0-beta.18 to v3.0.0 stable and switches Linux to GTK4 / WebKitGTK 6.0. It adds .deb and .rpm packages with an AppArmor profile, and carries the backend embedded-file save, File > Open Recent, native row menus, document-dependent menu enable state, About as an in-window dialog, and the Linux splash and app-id fixes.

anovik reviewed this same SHA (35c1d4c) and left 10 inline comments. I do not repeat them here. Under "anovik's comments, checked" each one gets a status (PoC-confirmed, confirmed by source read, or demoted) and a severity, so the threads can be triaged. What this review adds is F1, a packaging issue nobody has raised yet, and the claim-by-claim check of the description. No must-fix. F1 reproduces every time, but it only affects systems the README already lists as unsupported. Each of anovik's items either has a narrow trigger or depends on platform behaviour I could not run. Findings are keyed F1, F2, ... and anovik's comments A1-A10 in the order they were posted, so none of them autolink to an issue.

The PoCs ran on macOS (arm64) and in Docker. The code for each is in the appendix (P1-P5), with the command to run it.

F1 [should-fix] build/linux/nfpm/nfpm.yaml line 41

The .deb depends on libgtk-4-1 and libwebkitgtk-6.0-4 with no version. Debian 12 has both: libgtk-4-1 4.8.3+ds-2+deb12u1 and libwebkitgtk-6.0-4 2.50.6-1~deb12u2 (checked with madison for bookworm). So apt install ./unidoc-pdf-debugger-<ver>-linux-amd64.deb succeeds there. The binary then cannot work: Wails v3.0.0's GTK4 backend (pkg/application/linux_cgo.go) calls gtk_file_dialog_new, gtk_file_dialog_open and the other GtkFileDialog functions, which GTK added in 4.10. The build host's glibc is also newer (2.39 against Debian 12's 2.36). The README and CHANGELOG say Debian 12 is unsupported, but the package lets it install anyway, so the user's first sign of a problem is a broken app rather than an apt error. The description's own item 13 makes the same point about glibc and ubuntu-latest. Ubuntu 22.04 is in the same position: jammy-updates carries libwebkitgtk-6.0-4 2.50.4-0ubuntu0.22.04.1 and libgtk-4-1 4.6.9, so the .deb installs there too (checked with madison; not run in Docker).

PoC-confirmed in Docker, linux/amd64 (Appendix P1). On ubuntu:24.04 I built a small cgo binary that calls gtk_file_dialog_new, the same as Wails' backend. I packed it twice with dpkg-deb: once with this PR's Depends line verbatim, once with the fix below. These are not packages built by wails3 tool package from an edited nfpm.yaml, so the yaml snippet below is untested as yaml. Then I installed each with apt:

[Debian GNU/Linux 12 (bookworm)] head.deb: apt install OK (libgtk-4-1 4.8.3+ds-2+deb12u1, libc6 2.36-9+deb12u14)
  start:  rc=0 started; runtime gtk 4.8.3
  dialog: rc=127 poc-gtkdep: symbol lookup error: poc-gtkdep: undefined symbol: gtk_file_dialog_new
[Debian GNU/Linux 12 (bookworm)] fixed.deb: apt install REFUSED
[Ubuntu 24.04.5 LTS] head.deb: apt install OK (libgtk-4-1 4.14.5+ds-0ubuntu0.10)
  dialog: rc=0 file dialog created

The PoC binary uses lazy binding and needs only GLIBC_2.34. It starts, then dies at the first file dialog. I did not inspect the real app binary's binding mode or glibc symbol versions, so on Debian 12 it either fails at start or at the first Open/Save. Both are broken.

Suggested fix: state the build host's floor, so apt refuses an unsupported system:

depends:
  - libc6 (>= 2.39)
  - libgtk-4-1 (>= 4.14)
  - libwebkitgtk-6.0-4
...
overrides:
  rpm:
    depends:
      - glibc >= 2.39
      - gtk4 >= 4.14
      - webkitgtk6.0

4.14 is what CI and releases build against (wails3 doctor in the ubuntu-24.04 job reports gtk4 4.14.5). The hard minimum for GtkFileDialog is 4.10. With these depends, Ubuntu 24.04 (4.14.5) and Debian 13 (4.18) still install. The rpm lines are source-read only; I did not test the rpm override.

anovik's comments, checked

A1 [should-fix] frontend/src/components/navigatorHooks.ts:162 - ContextMenu key arms two tokens

Consequence confirmed by PoC (Appendix P2); the premise is not verified. I ran the real useRowContextMenu in jsdom: a ContextMenu keydown on a row, then a second contextmenu at the same row, then a click carrying the first menu's token. Result: head: tokens differ | acted=[]. The second event mints a new token, and the click from the first menu is dropped. Whether WebView2 and WebKitGTK actually send that second event after the hook's own one can't be reproduced from macOS or in jsdom. The PR description says Windows and Linux row menus were not tried by hand.

With a one-line change, the click works whichever menu it comes from (fixed: tokens same | acted=[reveal:obj:7:0]): when the row being re-armed is the node already pending, reuse its token.

const prev = pending.current;
const token = prev && prev.nodeId === hit.nodeId ? prev.token : String(++nextRowMenuToken);

That stops the dropped action, not the second menu. Stopping the second menu needs anovik's platform check and someone to try it on Windows and Linux.

A2 [should-fix] main.go:988 - Linux splash wait can pass the 30s timeout

PoC-confirmed (Appendix P3). I copied the Linux mainWindowReady closure and the WindowLoadFinished handler verbatim into a test, against the real Scheduler and the package's fake clock. The only change is time.AfterFunc becoming clock.AfterFunc. Outcome per variant (mainShown | timeout | forceQuit):

• main ready at 1s, splash never loads: base, head and fixed all show the main window.
• main ready at 28.5s, splash never loads: base shows it; head mainShown=false timeout=true forceQuit=true; fixed shows it.
• main ready at 29s, splash loads at 31s: base shows it; head quits; fixed shows it.
• main ready at 29s, splash loads at 29.5s: all show it.

So a Linux launch whose main WebView becomes ready in the last 2s before the timeout, with the splash page not yet loaded, is quit 60s later even though it loaded. That window is narrow. If WindowLoadFinished turned out not to fire for a Hidden: true window on some WebKitGTK, every launch would take the 2s path, and the window would still be the last 2s before the timeout. Fix tested in the PoC: cap the wait to the time left before the timeout.

splashStart := time.Now() // next to NewScheduler
...
wait := min(2*time.Second, time.Until(splashStart.Add(30*time.Second-100*time.Millisecond)))
if wait <= 0 {
	splashScheduler.MainWindowReady()
	return
}
time.AfterFunc(wait, splashScheduler.MainWindowReady)

(30s is splashTimeoutMs. Exporting it, or a Scheduler.Deadline(), would keep the two from drifting apart.)

A3 [should-fix] internal/pdfservice/service.go:698 - drive-relative and stream names pass through

PoC-confirmed for the name (Appendix P4); the Windows dialog's handling is not verified. I called embeddedSaveName directly. ../../evil.sh, ..\..\evil.bat and C:\Windows\evil.bat come out as bare file names. C:evil.bat, D: and invoice.pdf:payload (an NTFS alternate data stream) come out unchanged. The function's doc says the dialog never starts from a name that points into another directory, and the name comes from the PDF. What the Windows save dialog then does with C:evil.bat (anovik: resolve it against drive C's current directory) I could not observe from macOS. The suggested fix below passes all seven cases. It replaces : instead of dropping everything before it, so a macOS name like Notes 10:30.txt keeps its full text:

if i := strings.LastIndexAny(name, `/\`); i >= 0 {
	name = name[i+1:]
}
name = strings.ReplaceAll(name, ":", "_")

A row for C:evil.bat in TestSaveEmbeddedFileSeedsTheDialogWithTheLastPathElement would keep it fixed.

A4 [should-fix] main.go:617 - About does not bring the main window forward

Confirmed by source read. The removed openAboutWindow called Restore() and Focus(). The new handlers only emit about:open, which the main window's frontend receives whether or not the window is visible. Not run.

A5 [nit] internal/recentfiles/recentfiles.go:146 - a second atomic-write implementation

Confirmed by source read. List.Save has neither the rename retry nor the removeStaleTemps that updatecheck.Cache.write has. Neither Save nor Cache.write fsyncs, so the word "fsync" in anovik's main.go:824 comment does not apply. The file is small, and losing one update writes the list again on the next open. Nit.

A6 [should-fix] internal/recentfiles/recentfiles.go:64 - one file listed twice under different case

PoC-confirmed (Appendix P5) on a default case-insensitive APFS volume, using the real List. Add(".../Docs/a.pdf") then Add(".../docs/A.pdf"), with os.SameFile true for the two, gives entries=2 labels=["A.pdf" "a.pdf"], and Remove of one leaves the other. The base names differ in case, so Labels does not add the parent folder either. Fixed (strings.EqualFold in without when GOOS is darwin or windows): entries=1. On macOS that would merge two distinct files only on a case-sensitive volume, which is rare.

A7 [should-fix] main.go:824 - one save and one menu-bar rebuild per opened file

Confirmed by source read. RecordRecentFile runs inside each per-file goroutine of openFileAndEmitWithWarning, so a batch of N fires N Save calls and N rebuilds, serialised only by recentMu. The Wails v3.0.0 source shows each rebuild redoes the whole menu bar on every platform. On macOS, Menu.Update clears and rebuilds the NSMenu in InvokeSync. On Windows, setMenu builds a new HMENU and frees the old one. On Linux, setMenu runs menu.Update(), which does g_menu_remove_all and re-adds every item. Not measured.

A8 [nit] frontend/src/App.jsx:146 - first state is never sent

Confirmed; demoted to nit on reachability. The PR's own App.menuState.test.tsx case "nothing is reported while no document is open" asserts exactly the behaviour anovik describes: a fresh mount sends nothing, so after a reload the Go menu keeps its last state. The fix (useRef(null)) would flip that test. In a production build, though, both reload keys are taken: App.jsx preventDefaults F5 and Cmd/Ctrl+R on keydown wherever focus is. That leaves dev reloads and any other WebView reload path.

A9 [nit] frontend/src/components/TreePanel.tsx:369 - second lookup by backendId

Demoted by source read. The only same-tab write of treeDataRef that drops nodes is the effect keyed on rootNode, rootChildren and activeTabId. A reload gives the tab a new id: reloadDocument.ts dispatches the reopened document under result.tabId, and OPEN_DOCUMENT replaces the tab with that filePath and makes the new id active. A background reload leaves the active tab alone. The closeMenu effect on activeTabId therefore clears pending before any click. A copy that does nothing after a reload would come from that deliberate forget, and carrying objectRef in pending would not change it. What remains is the simplification: carry objectRef and skip the second walk.

A10 [nit] frontend/src/components/TreePanel.tsx:38 - doc comment does not match the palette

Confirmed by source read. CommandPalette.tsx:111 shows path.slice(0, -1).join(' > ') (target dropped), while Copy path joins the whole path. The CHANGELOG line "the same breadcrumb the Cmd/Ctrl+K palette shows" is off for the same reason.

PR description verification

• "Go module v3.0.0, @wailsio/runtime exact 3.0.0, Go 1.27.3, vite ^8.2.4" - confirmed (go.mod, frontend/package.json). The GO-2026-6809 reference was not verified; I did not check the vulnerability database.
• "Wails checks each emit against the type the event was registered with and silently cancels a mismatch" - confirmed: validateCustomEvent in pkg/application/events.go cancels the event. The error does go to the app's error handler, so it is silent in the UI rather than in the log. Every Go emit site passes the exact registered type.
• "the bytes ... overflowed the 64 MiB request cap" - confirmed: maxAssembledBytes = 64 * 1024 * 1024 in transport_http.go. 50 MiB as base64 is about 69.9 MB.
• "all 22 app events are registered in init()" - partially confirmed: events.go registers 23 (row-menu and about included), plus the update service's one. The binding generator reports 24.
• "events_registration_test.go pins each event to the exact type its emitters use, so a wrong registration fails the test" - partially confirmed: it compares registrations with a hand-written table, not with the emit sites. If an emit site's type changes and the table does not, the test still passes and the event is cancelled at runtime.
• "The document:warning listener in App.jsx had no emitter" - confirmed: at 077196f the name occurs only in that listener and in a test's string list.
• "test-setup.ts stubs the generated eventcreate.js" - confirmed.
• "Read errors return before any dialog is shown, and cancel is silent ... ../../evil.sh suggests evil.sh" - confirmed. Drive-relative names are not covered (A3). "mocks in 25 test files": 26 by my count.
• Open Recent: "up to 10 paths ... saved atomically", "missing or damaged file starts an empty list", "Reload and Compare never record", "& / _ doubled" - confirmed by source. Wails v3.0.0 does no mnemonic escaping of its own, so the doubling is needed. Case variants are not merged (A6).
• "The Wails runtime opens a menu from the --custom-contextmenu CSS property" - confirmed (@wailsio/runtime 3.0.0 dist/contextmenu.js). "Go never reads anything from the page-supplied data" - confirmed: the token is only echoed back. "On Windows, Wails opens every context menu at the mouse pointer" - not verified.
• "Section headers in the File and app menus (macOS 14+)" - confirmed: AddSectionHeader is a no-op elsewhere.
• "build/windows/info.json keys the version block under 0409 to match the v3.0.0 template" - confirmed against internal/commands/updatable_build_assets/windows/info.json.tmpl.
• "CI ... installs it, checks aa-status, asserts the runner still has the restriction on, and runs the boot smoke against the installed binary" - confirmed from the ubuntu-24.04 job log: Setting up unidoc-pdf-debugger (0.0.0~ci), and the app process shows label="unidoc-pdf-debugger (unconfined)", so the packaged profile is attached. The boot smoke passed.
• "wails3 doctor reports no issues: ... gtk4 4.14.5, webkitgtk-6.0 2.52.6" - confirmed from the same log.
• "CI green on all three runners" - confirmed (gh pr checks).
• "Ubuntu 22.04, Debian 12 and RHEL 9 no longer run the app" - partially confirmed: true of the binary, but the .deb still installs on Debian 12 and on Ubuntu 22.04 with jammy-updates (F1).
• "[the splash] starts hidden, and shows on WindowLoadFinished; the main window waits for that (2s at most)" - not verified. Nothing here shows that WindowLoadFinished fires for a Hidden: true window, and the Linux boot smoke would pass on the 2s fallback alone. The timeout interaction is A2.
• "main.go sets Linux.ApplicationID ... desktop entry gets a matching StartupWMClass" - confirmed. Wails also uses that id as the GTK program name, which sets the Wayland app_id (application_linux_appid.go). The dock icon effect is not verified.
• "The update checker still offers Linux users the tarball, since it matches the -linux-amd64.tar.gz suffix" - confirmed: assetFor uses HasSuffix, so the .deb and .rpm can never be picked.
• "On Windows, Ctrl+R with no document open now reaches WebView2 ... If WebView2 reloads the page" - not verified, and probably moot: App.jsx preventDefaults Cmd/Ctrl+R and F5 on keydown in every state.
• "go mod tidy -diff is clean" - confirmed (re-run).
• "go test -race -count=1 on the root package, internal/recentfiles, internal/pdfservice and internal/updateservice passed" - confirmed on macOS arm64 (the root package needs a built frontend/dist). "All 75 suites passed" was not re-run in full.

Appendix: PoC code

Each PoC is given in full, with the command to run it and its output. They were not committed to the PR branch.

P1 - F1: the .deb installs on Debian 12 and the binary fails there

Three files in one directory (poc-deb/). Run from that directory:

D=$PWD
docker run --rm --platform linux/amd64 -v "$D:/poc" ubuntu:24.04 /poc/build.sh
for v in head fixed; do docker run --rm --platform linux/amd64 -v "$D:/poc" debian:bookworm-slim /poc/try.sh $v; done
docker run --rm --platform linux/amd64 -v "$D:/poc" ubuntu:24.04 /poc/try.sh head

src/main.go:

// PoC for PR 40 F1: a cgo binary that, like the Wails v3.0.0 GTK4 backend
// (pkg/application/linux_cgo.go), calls the GTK 4.10 GtkFileDialog API.
package main

// #cgo pkg-config: gtk4
// #include <gtk/gtk.h>
// #include <stdio.h>
// static void* newDialog(void) { return gtk_file_dialog_new(); }
// static const char* ver(void) { static char b[32]; snprintf(b, sizeof b, "%u.%u.%u", gtk_get_major_version(), gtk_get_minor_version(), gtk_get_micro_version()); return b; }
import "C"

import (
	"fmt"
	"os"
)

func main() {
	fmt.Println("started; runtime gtk", C.GoString(C.ver()))
	if len(os.Args) > 1 {
		C.newDialog()
		fmt.Println("file dialog created")
	}
}

build.sh:

#!/bin/sh
# Stage 1, runs in ubuntu:24.04 (the release build host): build the PoC binary
# and pack it twice - with the PR's nfpm depends verbatim (head) and with
# versioned depends (fixed). Writes only into /poc (the bind-mounted scratch dir).
set -eu
export DEBIAN_FRONTEND=noninteractive
apt-get update -qq >/dev/null
apt-get install -y -qq --no-install-recommends golang-go gcc libc6-dev libgtk-4-dev pkg-config dpkg-dev binutils >/dev/null
cd /poc/src
[ -f go.mod ] || go mod init poc >/dev/null 2>&1
GOFLAGS=-buildvcs=false go build -o /poc/poc .
echo "build host: $(ldd --version | head -1); gtk $(pkg-config --modversion gtk4)"
echo "highest GLIBC symbol version the binary needs: $(objdump -T /poc/poc | grep -o 'GLIBC_[0-9.]*' | sort -uV | tail -1)"
echo "gtk_file_dialog dynamic refs: $(objdump -T /poc/poc | grep -c gtk_file_dialog)"
echo "BIND_NOW flags: $(readelf -d /poc/poc | grep -E 'FLAGS' | tr -s ' ' | tr '\n' ' ')"
arch=$(dpkg --print-architecture)
mk() {
  name=$1 depends=$2
  root=/poc/pkg-$name
  mkdir -p "$root/DEBIAN" "$root/usr/bin"
  cp /poc/poc "$root/usr/bin/poc-gtkdep"
  printf 'Package: poc-gtkdep\nVersion: 1.0\nArchitecture: %s\nMaintainer: poc <poc@example.invalid>\nDepends: %s\nDescription: PR 40 depends PoC\n' "$arch" "$depends" > "$root/DEBIAN/control"
  dpkg-deb --build --root-owner-group "$root" "/poc/$name.deb" >/dev/null
  echo "$name.deb Depends: $(dpkg-deb -f /poc/$name.deb Depends)"
}
# build/linux/nfpm/nfpm.yaml at 35c1d4c: depends: [libgtk-4-1, libwebkitgtk-6.0-4]
mk head 'libgtk-4-1, libwebkitgtk-6.0-4'
mk fixed 'libc6 (>= 2.39), libgtk-4-1 (>= 4.14), libwebkitgtk-6.0-4'

try.sh:

#!/bin/sh
# Stage 2: install a PoC .deb with apt on the target image and run the binary.
# $1 = head|fixed
set -u
export DEBIAN_FRONTEND=noninteractive
apt-get update -qq >/dev/null
. /etc/os-release
if apt-get install -y -qq --no-install-recommends "/poc/$1.deb" >/tmp/apt.log 2>&1; then
  echo "[$PRETTY_NAME] $1.deb: apt install OK (libgtk-4-1 $(dpkg-query -W -f '${Version}' libgtk-4-1), libc6 $(dpkg-query -W -f '${Version}' libc6))"
  out=$(poc-gtkdep 2>&1); echo "  start:  rc=$? $out"
  out=$(poc-gtkdep dialog 2>&1); echo "  dialog: rc=$? $out"
else
  echo "[$PRETTY_NAME] $1.deb: apt install REFUSED"
  grep -E "Depends|unmet|^E:" /tmp/apt.log | head -4 | sed 's/^/  /'
fi

Output (Docker's platform-mismatch warnings and debconf noise removed):

build host: ldd (Ubuntu GLIBC 2.39-0ubuntu8.9) 2.39; gtk 4.14.5
highest GLIBC symbol version the binary needs: GLIBC_2.34
gtk_file_dialog dynamic refs: 1
BIND_NOW flags: 
head.deb Depends: libgtk-4-1, libwebkitgtk-6.0-4
fixed.deb Depends: libc6 (>= 2.39), libgtk-4-1 (>= 4.14), libwebkitgtk-6.0-4
[Debian GNU/Linux 12 (bookworm)] head.deb: apt install OK (libgtk-4-1 4.8.3+ds-2+deb12u1, libc6 2.36-9+deb12u14)
  start:  rc=0 started; runtime gtk 4.8.3
  dialog: rc=127 started; runtime gtk 4.8.3
poc-gtkdep: symbol lookup error: poc-gtkdep: undefined symbol: gtk_file_dialog_new
[Debian GNU/Linux 12 (bookworm)] fixed.deb: apt install REFUSED
  E: Unable to correct problems, you have held broken packages.
[Ubuntu 24.04.5 LTS] head.deb: apt install OK (libgtk-4-1 4.14.5+ds-0ubuntu0.10, libc6 2.39-0ubuntu8.9)
  start:  rc=0 started; runtime gtk 4.14.5
  dialog: rc=0 started; runtime gtk 4.14.5
file dialog created

P2 - A1: a second contextmenu at the same row drops the first menu's action

Two files in frontend/src/components/. poc_pr40_navigatorHooks.fixed.ts is a copy of navigatorHooks.ts with this one change:

--- navigatorHooks.ts
+++ poc_pr40_navigatorHooks.fixed.ts
@@ -146,7 +146,10 @@
     const hit = id ? resolveRef.current(id) : null;
     if (!id || !hit) return;
     treeRef.current?.select(id);
-    const token = String(++nextRowMenuToken);
+    // Re-arming the node already pending keeps its token, so a second contextmenu
+    // for one key press does not orphan the first menu.
+    const prev = pending.current;
+    const token = prev && prev.nodeId === hit.nodeId ? prev.token : String(++nextRowMenuToken);
     pending.current = { token, nodeId: hit.nodeId };
     container.style.setProperty('--custom-contextmenu', hit.menu);
     container.style.setProperty('--custom-contextmenu-data', token);

poc_pr40_rowmenu_double.test.tsx:

/**
 * PoC for PR 40 (anovik's navigatorHooks.ts:162 comment): if one ContextMenu
 * key press yields two contextmenu events at the same row (the hook's own
 * synthetic one on keydown, then the WebView's), the second arms a new token
 * and a click in the first menu is dropped.
 *
 * head  = the real useRowContextMenu at 35c1d4c (navigatorHooks.ts).
 * fixed = poc_pr40_navigatorHooks.fixed.ts: the same file with one change,
 *         re-arming the node already pending keeps its token.
 * base  = 077196f drew its own React menu and had no tokens; a second event
 *         reopened the same menu (source-read, not run here).
 *
 * Not modelled: whether the WebView really sends that second event. That is
 * platform behaviour (WebView2 on keyup, WebKitGTK via popup-menu) and is not
 * reproducible in jsdom; the second event is fired by hand.
 *
 * Run: cd frontend && npx vitest run src/components/poc_pr40_rowmenu_double.test.tsx
 */
import { useRef } from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { describe, test, expect } from 'vitest';
import { useRowContextMenu as headHook } from './navigatorHooks';
import { useRowContextMenu as fixedHook } from './poc_pr40_navigatorHooks.fixed';
import { armedRowMenu, clickRowMenuItem } from './rowMenuTestUtils';

type Hook = typeof headHook;

function Panel({ hook, acted }: { hook: Hook; acted: string[] }) {
  const treeRef = useRef(undefined);
  const { onContextMenu, onKeyDown } = hook(
    treeRef,
    '[data-row-id]',
    'data-row-id',
    (rowId) => ({ menu: 'row-reveal', nodeId: `obj:${rowId}:0` }),
    (action, nodeId) => acted.push(`${action}:${nodeId}`),
  );
  return (
    <div onContextMenu={onContextMenu} onKeyDown={onKeyDown}>
      <div role="treeitem" tabIndex={0} data-testid="item">
        <span data-row-id="7">row 7</span>
      </div>
    </div>
  );
}

function run(hook: Hook): string {
  const acted: string[] = [];
  const { unmount } = render(<Panel hook={hook} acted={acted} />);
  const item = screen.getByTestId('item');
  const row = screen.getByText('row 7');
  // One ContextMenu key press: the hook dispatches its own contextmenu ...
  fireEvent.keyDown(item, { key: 'ContextMenu' });
  const first = armedRowMenu(row)?.token ?? '';
  // ... and the WebView sends one too.
  fireEvent.contextMenu(row);
  const second = armedRowMenu(row)?.token ?? '';
  // The user picks the item in the menu that opened first.
  clickRowMenuItem('reveal', first);
  unmount();
  return `tokens ${first === second ? 'same' : 'differ'} | acted=[${acted.join(',')}]`;
}

describe('ContextMenu key with a second native contextmenu', () => {
  test('head drops the first menu action, fixed keeps it', () => {
    const got = `head: ${run(headHook)} || fixed: ${run(fixedHook)}`;
    console.log(got);
    expect(got).toBe('head: tokens same | acted=[reveal:obj:7:0] || fixed: tokens same | acted=[reveal:obj:7:0]');
  });
});

Output (the assertion states the correct behaviour, so head failing is the reproduction):

head: tokens differ | acted=[] || fixed: tokens same | acted=[reveal:obj:7:0]

P3 - A2: Linux splash wait past the 30s timeout

internal/splash/poc_pr40_linux_ready_test.go (uses the package's fakeClock from splash_test.go):

package splash

// PoC for PR 40 (anovik's main.go:988 comment): the Linux splash wiring can
// push MainWindowReady past the 30s timeout, after which the main window is
// never shown and the 60s force-quit closes an app that loaded.
//
// The Linux closure and the WindowLoadFinished handler are transplanted
// verbatim from main.go lines 1062-1082 at 35c1d4c, with one substitution:
// time.AfterFunc becomes clock.AfterFunc so the fake clock drives it. The
// Scheduler is the real one (NewScheduler's 400ms floor and 30s timeout).
//
//   base  = 077196f: WindowRuntimeReady calls MainWindowReady directly.
//   head  = 35c1d4c: wait up to 2s for the splash's WindowLoadFinished.
//   fixed = head, with the 2s wait capped to the time left before the timeout.
//
// Run: go test ./internal/splash -run TestPoCPR40 -v

import (
	"fmt"
	"sync/atomic"
	"testing"
	"time"
)

type pocWiring struct {
	mainWindowReady    func()
	windowLoadFinished func()
}

func pocBase(s *Scheduler, _ *fakeClock, _ time.Time) pocWiring {
	return pocWiring{mainWindowReady: s.MainWindowReady, windowLoadFinished: func() {}}
}

func pocHead(splashScheduler *Scheduler, clock *fakeClock, _ time.Time) pocWiring {
	var splashLoaded, mainReady atomic.Bool
	return pocWiring{
		windowLoadFinished: func() {
			if splashScheduler.Shown() {
				// splashWindow.Show()
			}
			splashLoaded.Store(true)
			if mainReady.Load() {
				splashScheduler.MainWindowReady()
			}
		},
		mainWindowReady: func() {
			mainReady.Store(true)
			if splashLoaded.Load() {
				splashScheduler.MainWindowReady()
				return
			}
			clock.AfterFunc(2*time.Second, splashScheduler.MainWindowReady)
		},
	}
}

func pocFixed(splashScheduler *Scheduler, clock *fakeClock, created time.Time) pocWiring {
	var w pocWiring
	var splashLoaded, mainReady atomic.Bool
	w.windowLoadFinished = func() {
		splashScheduler.Shown()
		splashLoaded.Store(true)
		if mainReady.Load() {
			splashScheduler.MainWindowReady()
		}
	}
	w.mainWindowReady = func() {
		mainReady.Store(true)
		if splashLoaded.Load() {
			splashScheduler.MainWindowReady()
			return
		}
		// Never wait past the splash timeout: leave 100ms of the 30s budget.
		wait := min(2*time.Second, created.Add(splashTimeoutMs*time.Millisecond-100*time.Millisecond).Sub(clock.Now()))
		if wait <= 0 {
			splashScheduler.MainWindowReady()
			return
		}
		clock.AfterFunc(wait, splashScheduler.MainWindowReady)
	}
	return w
}

// run drives one launch: the main WebView reports ready at mainAt, the splash
// page finishes loading at splashAt (negative: never). It returns the joined
// outcome after 95s of virtual time, so the 60s force-quit would have fired.
func run(variant func(*Scheduler, *fakeClock, time.Time) pocWiring, mainAt, splashAt time.Duration) string {
	start := time.Unix(0, 0)
	clock := newFakeClock(start)
	var dismissed, timedOut, forceQuit atomic.Bool
	var splashFailed atomic.Bool
	s := NewScheduler(clock, func() {
		dismissed.Store(true) // onSplashDismiss: mainWindow.Show()
	}, func() {
		splashFailed.Store(true)
		timedOut.Store(true)
		clock.AfterFunc(60*time.Second, func() {
			if splashFailed.Load() {
				forceQuit.Store(true) // app.Quit()
			}
		})
	})
	w := variant(s, clock, start)
	type ev struct {
		at time.Duration
		fn func()
	}
	evs := []ev{{mainAt, w.mainWindowReady}}
	if splashAt >= 0 {
		evs = append(evs, ev{splashAt, w.windowLoadFinished})
	}
	if len(evs) == 2 && evs[1].at < evs[0].at {
		evs[0], evs[1] = evs[1], evs[0]
	}
	// fakeClock.Advance fires every due timer in registration order and skips
	// timers registered during the same call, so time moves in 50ms steps.
	var now time.Duration
	step := func(to time.Duration) {
		for now < to {
			d := min(50*time.Millisecond, to-now)
			clock.Advance(d)
			now += d
		}
	}
	for _, e := range evs {
		step(e.at)
		e.fn()
	}
	step(95 * time.Second)
	return fmt.Sprintf("mainShown=%v timeout=%v forceQuit=%v", dismissed.Load(), timedOut.Load(), forceQuit.Load())
}

func TestPoCPR40LinuxSplashReady(t *testing.T) {
	ok := "mainShown=true timeout=false forceQuit=false"
	bad := "mainShown=false timeout=true forceQuit=true"
	cases := []struct {
		name               string
		mainAt, splashAt   time.Duration
		wantBase, wantHead string
	}{
		{"fast launch, splash loads after main", 1 * time.Second, 1200 * time.Millisecond, ok, ok},
		{"fast launch, splash never loads", 1 * time.Second, -1, ok, ok},
		{"slow launch, main ready 27.9s, splash never loads", 27900 * time.Millisecond, -1, ok, ok},
		{"slow launch, main ready 28.5s, splash never loads", 28500 * time.Millisecond, -1, ok, bad},
		{"slow launch, main ready 29s, splash loads at 31s", 29 * time.Second, 31 * time.Second, ok, bad},
		{"slow launch, main ready 29s, splash loads at 29.5s", 29 * time.Second, 29500 * time.Millisecond, ok, ok},
	}
	for _, c := range cases {
		for _, v := range []struct {
			name string
			fn   func(*Scheduler, *fakeClock, time.Time) pocWiring
			want string
		}{{"base", pocBase, c.wantBase}, {"head", pocHead, c.wantHead}, {"fixed", pocFixed, ok}} {
			got := v.name + " | " + c.name + " | " + run(v.fn, c.mainAt, c.splashAt)
			want := v.name + " | " + c.name + " | " + v.want
			t.Log(got)
			if got != want {
				t.Errorf("\n got  %s\n want %s", got, want)
			}
		}
	}
}

Output:

base | fast launch, splash loads after main | mainShown=true timeout=false forceQuit=false
head | fast launch, splash loads after main | mainShown=true timeout=false forceQuit=false
fixed | fast launch, splash loads after main | mainShown=true timeout=false forceQuit=false
base | fast launch, splash never loads | mainShown=true timeout=false forceQuit=false
head | fast launch, splash never loads | mainShown=true timeout=false forceQuit=false
fixed | fast launch, splash never loads | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 27.9s, splash never loads | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 27.9s, splash never loads | mainShown=true timeout=false forceQuit=false
fixed | slow launch, main ready 27.9s, splash never loads | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 28.5s, splash never loads | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 28.5s, splash never loads | mainShown=false timeout=true forceQuit=true
fixed | slow launch, main ready 28.5s, splash never loads | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 29s, splash loads at 31s | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 29s, splash loads at 31s | mainShown=false timeout=true forceQuit=true
fixed | slow launch, main ready 29s, splash loads at 31s | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 29s, splash loads at 29.5s | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 29s, splash loads at 29.5s | mainShown=true timeout=false forceQuit=false
fixed | slow launch, main ready 29s, splash loads at 29.5s | mainShown=true timeout=false forceQuit=false

P4 - A3: drive-relative and stream names pass through embeddedSaveName

internal/pdfservice/poc_pr40_embedded_name_test.go:

package pdfservice

// PoC for PR 40 (anovik's service.go:698 comment): does the suggested save
// name still point outside the folder the dialog shows?
//
// head  = embeddedSaveName at 35c1d4c (internal/pdfservice/service.go), called directly.
// base  = 077196f: the frontend passed selected.name to SaveBytesToFile verbatim.
// fixed = head plus ':' replaced, so no drive prefix or NTFS stream survives.
//
// Run: go test ./internal/pdfservice -run TestPoCPR40EmbeddedSaveName -v

import (
	"fmt"
	"strings"
	"testing"
)

func pocBaseSaveName(name string) string { return name }

func pocFixedSaveName(name string) string {
	if i := strings.LastIndexAny(name, `/\`); i >= 0 {
		name = name[i+1:]
	}
	name = strings.ReplaceAll(name, ":", "_")
	name = strings.TrimSpace(name)
	if name == "" || name == "." || name == ".." {
		return "embedded-file"
	}
	return name
}

// escapes reports whether a suggested name can resolve outside the folder the
// Windows save dialog shows: it carries a separator or a drive/stream colon.
func escapes(name string) bool { return strings.ContainsAny(name, `/\:`) }

func TestPoCPR40EmbeddedSaveName(t *testing.T) {
	cases := []struct{ in, want string }{
		{"report.pdf", "report.pdf"},
		{"../../evil.sh", "evil.sh"},
		{`..\..\evil.bat`, "evil.bat"},
		{`C:\Windows\evil.bat`, "evil.bat"},
		{"C:evil.bat", "C_evil.bat"},
		{"D:", "D_"},
		{"invoice.pdf:payload", "invoice.pdf_payload"},
	}
	for _, c := range cases {
		for _, v := range []struct {
			name string
			fn   func(string) string
		}{{"base", pocBaseSaveName}, {"head", embeddedSaveName}, {"fixed", pocFixedSaveName}} {
			out := v.fn(c.in)
			got := fmt.Sprintf("%s | %q -> %q | escapes=%v", v.name, c.in, out, escapes(out))
			t.Log(got)
			if v.name == "fixed" {
				if want := fmt.Sprintf("fixed | %q -> %q | escapes=false", c.in, c.want); got != want {
					t.Errorf("got  %s\nwant %s", got, want)
				}
			}
		}
	}
}

Output:

base | "report.pdf" -> "report.pdf" | escapes=false
head | "report.pdf" -> "report.pdf" | escapes=false
fixed | "report.pdf" -> "report.pdf" | escapes=false
base | "../../evil.sh" -> "../../evil.sh" | escapes=true
head | "../../evil.sh" -> "evil.sh" | escapes=false
fixed | "../../evil.sh" -> "evil.sh" | escapes=false
base | "..\\..\\evil.bat" -> "..\\..\\evil.bat" | escapes=true
head | "..\\..\\evil.bat" -> "evil.bat" | escapes=false
fixed | "..\\..\\evil.bat" -> "evil.bat" | escapes=false
base | "C:\\Windows\\evil.bat" -> "C:\\Windows\\evil.bat" | escapes=true
head | "C:\\Windows\\evil.bat" -> "evil.bat" | escapes=false
fixed | "C:\\Windows\\evil.bat" -> "evil.bat" | escapes=false
base | "C:evil.bat" -> "C:evil.bat" | escapes=true
head | "C:evil.bat" -> "C:evil.bat" | escapes=true
fixed | "C:evil.bat" -> "C_evil.bat" | escapes=false
base | "D:" -> "D:" | escapes=true
head | "D:" -> "D:" | escapes=true
fixed | "D:" -> "D_" | escapes=false
base | "invoice.pdf:payload" -> "invoice.pdf:payload" | escapes=true
head | "invoice.pdf:payload" -> "invoice.pdf:payload" | escapes=true
fixed | "invoice.pdf:payload" -> "invoice.pdf_payload" | escapes=false

P5 - A6: one file listed twice under different case

internal/recentfiles/poc_pr40_case_test.go (run on a case-insensitive volume):

package recentfiles

// PoC for PR 40 (anovik's recentfiles.go:64 comment): on a case-insensitive
// file system one file can be listed twice in Open Recent.
//
//   head  = the real List at 35c1d4c (Add/Remove/Labels).
//   fixed = Add with entries compared by strings.EqualFold on darwin/windows.
//   base  = 077196f had no Open Recent; not applicable.
//
// Run (on a default, case-insensitive APFS or NTFS volume):
//   go test ./internal/recentfiles -run TestPoCPR40 -v

import (
	"fmt"
	"os"
	"path/filepath"
	"runtime"
	"strings"
	"testing"
)

func pocFixedAdd(paths []string, path string) []string {
	p := normalize(path)
	same := func(a, b string) bool {
		if runtime.GOOS == "darwin" || runtime.GOOS == "windows" {
			return strings.EqualFold(a, b)
		}
		return a == b
	}
	out := []string{p}
	for _, q := range paths {
		if !same(q, p) {
			out = append(out, q)
		}
	}
	return out
}

func TestPoCPR40CaseVariants(t *testing.T) {
	dir := t.TempDir()
	if err := os.MkdirAll(filepath.Join(dir, "Docs"), 0o755); err != nil {
		t.Fatal(err)
	}
	a := filepath.Join(dir, "Docs", "a.pdf")
	b := filepath.Join(dir, "docs", "A.pdf")
	if err := os.WriteFile(a, []byte("%PDF"), 0o644); err != nil {
		t.Fatal(err)
	}
	ia, errA := os.Stat(a)
	ib, errB := os.Stat(b)
	if errA != nil || errB != nil || !os.SameFile(ia, ib) {
		t.Skipf("volume is case-sensitive here (%v, %v); the PoC needs a case-insensitive one", errA, errB)
	}

	l := Load(filepath.Join(dir, "recent.json"))
	l.Add(a)
	l.Add(b)
	head := l.Paths()
	l.Remove(a)
	afterRemove := l.Paths()

	fixed := pocFixedAdd(pocFixedAdd(nil, a), b)

	got := fmt.Sprintf("head entries=%d labels=%q afterRemove=%d | fixed entries=%d",
		len(head), Labels(head), len(afterRemove), len(fixed))
	t.Log(got)
	want := `head entries=1 labels=["A.pdf"] afterRemove=0 | fixed entries=1`
	if got != want {
		t.Errorf("\n got  %s\n want %s", got, want)
	}
}

Output (head fails the correct-behaviour assertion):

got  head entries=2 labels=["A.pdf" "a.pdf"] afterRemove=1 | fixed entries=1
want head entries=1 labels=["A.pdf"] afterRemove=0 | fixed entries=1

Thanks for the second round. Re-review at 3288082: all 6 findings from the last round are fixed or mostly fixed. The splash race now has a readyPending flag and a test, About waits until the frontend has mounted, a missing file no longer matches a case variant and the Stat runs outside the lock, macOS and pointer presses no longer leave the key-menu flag set, and format characters are mapped in the save name. Nothing blocking is left. Inline comments cover a few smaller issues, mostly in the new fixes. The shutdown race in the Open Recent flush matters most.

Re-review at 0e1e7ab, covering the third round of fixes.

Fixed

• Save-name rule: only Bidi_Control, Zl and Zp are mapped now, and tests check that ZWNJ, ZWJ and the soft hyphen are kept.
• Key-menu echo guard: it is set only for the ContextMenu key.
• Case-variant race in Open Recent: lockSameAs rechecks under l.mu. I checked the lock order (saveMu → mu, no InvokeSync under the flusher lock) and found no deadlock.

Partly fixed

• Shutdown: recentFlusher.Close now stops the timer, waits for a save in progress and saves a pending change. Two gaps are left: a rebuild that started just before Close still runs during teardown, and Clear Menu and the missing-file removal bypass the flusher (inline).
• Batch coalescing: openFilesBatch holds the flusher again, and the frontend loops record once at the end. But the OnSecondInstanceLaunch loop (main.go:462, not in the diff so not inline) opens each path through openFileAndEmit without Hold/Release. pdfdebug big1.pdf big2.pdf big3.pdf against a running instance, with each file taking longer than 500 ms, still saves and rebuilds once per file. The CHANGELOG sentence "Opening several files at once saves the list and rebuilds the menu once ... however long each takes" names "a second launch" as one of the routes. Wrapping that loop in Hold/Release when there is more than one path would make it true.

The new inline comments are mostly about the frontend recording now happening after the loop. The ordering and lost-on-quit ones matter most.

Re-review at e35de9a, covering the fourth round of fixes.

Fixed

• Open Recent order: both loops await each recordRecentFile.
• Files lost when quitting mid-batch: each file is recorded as it opens, and Close saves what is still waiting.
• Rebuild after quit: the rebuild runs on the main thread and checks isClosed there, and Close runs on the main thread, so a late rebuild does nothing. I found no deadlock.
• Clear Menu and missing-file removal: both go through SaveNow.
• The openFilesBatch doc comment is back on its function, and the flusher gets save/rebuild in its constructor.
• Shutdown (main.go 886): Close stops the timer, waits for a save in progress and saves a pending change.

Partly fixed

• Batch coalescing (main.go 861/881) and the second launch: a launch carrying several paths is held now, but files that arrive one per process or event are not (inline).

Requesting changes for the startup nil dereference on Linux and the app-wide frontend hold that can stop Open Recent saving for the rest of the session (inline). Several of the new comments come from the frontend Begin/End protocol. Counting opens in flight on the Go side, which every open path goes through, would fix them and make that protocol unnecessary (see the comment on main.go:463).

Re-review at 194bdcb. Counting opens in flight in PDFService.OpenFile works well: all five findings from the last round are fixed, and nothing here blocks the merge.

Fixed

• Startup nil dereference: the second-launch callback no longer touches the flusher, and OpenFile checks the tracker for nil.
• One-file-per-process arrivals: every entry point goes through the in-flight count.
• Stuck frontend hold: the Begin/End/Reset protocol is gone.
• Progress dialog waiting on the save: nothing is awaited around the open loop now.
• SaveNow timing: the started/finished counters make it wait for its own save, and that test is deterministic now.
• Entry-point coverage (main.go 861).

I also checked that every OpenStarted gets exactly one OpenEnded via defer on every path, that the cap never changes inFlight (so a late end can't drive it negative), and that f.mu is never held across the save or InvokeSync. The flusher tests pass with -count=20 locally.

Partly fixed

• Slow batches (main.go 881): they no longer save once per file, but the 10 s cap still splits a long run into several saves (inline on recentflush.go:147).

The inline comments are about the cap's timing, the CHANGELOG wording, two timing-dependent tests and some smaller points.

Re-review at 6958ed2. The per-open cap fixes the cap timing and the stuck-open issue, the earlier timing-dependent tests are fixed, and slow batches now save once (main.go 881). I checked the new tracking: each open gets its own id with exactly one end, defer s.openStarted()() still ends on a recovered panic, arm() picks the newest open's deadline and re-arms when a newer open starts, an end after Close does nothing, and there are no lock-order or InvokeSync problems. I found no regressions in the code.

What's left

• The new test code is timing-dependent again. One test failed in a load run (inline). This is the one I'd fix before merge, since it will make CI unreliable.
• CHANGELOG / flusher doc wording: "saves once however long each file takes to open" followed by the 10 s exception still reads as a contradiction. A batch with one file slower than 10 s saves partway through. "however long each file takes, up to 10 seconds" (in recentflush.go:10-11 too) would match. Leaving the existing CHANGELOG thread open for that.
• Minor: an open's end re-arms the full delay even when it no longer holds the save (inline).

The declined service.go 195/196 threads are fine with me as reasoned. I'll leave them for you to resolve.